AI Automated Review

Full PR review.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: public_route_changes)

PR PR 377 Review: Filter merged/closed PRs in ingestMergeStateEvent and ingestReviewEvent

Recommendation: Approve

This PR correctly implements the guard described in issue PR 364, preventing merged/closed PRs from entering the PR-fix queue at ingest time. The fix is targeted, well-tested, and preserves existing auth controls.

Change-by-Change Findings

src/lib/pr-followup-ingestion.ts (core logic)

• Added prState?: string | null and prMergedAt?: string | null optional parameters to ingestReviewEvent and ingestMergeStateEvent
• Added early return guard if (opts.prMergedAt || opts.prState === "closed") return null; in both functions, placed before the bot-author check as expected per issue PR 364's suggested fix
• Extended PrFollowupEvent interface and processPrFollowupEvents to propagate these fields through the event pipeline
• The guard is positioned correctly: it short-circuits before any database write or queue enqueue operation, which is the intended behavior

src/app/api/pr-followup/sync/route.ts (pull-based ingestion)

• Passes pr.state and pr.merged_at as prState and prMergedAt in both review and merge-state event branches
• These values are sourced from the already-cached GithubPR object populated during sync, satisfying the issue's guidance to use cached state rather than making extra GitHub API calls

src/app/api/pr-followup/webhook/route.ts (webhook-based ingestion)

• Passes pr.state and pr.merged_at from the webhook payload in both pull_request_review and pull_request event handlers
• Signature verification remains intact (see below)

src/lib/pr-followup-ingestion.test.ts (new tests)

• Four new test cases covering merged (via prMergedAt) and closed (via prState) PRs for both ingestReviewEvent and ingestMergeStateEvent
• All tests assert result is null and client.items has length 0, confirming no queue entries are created for terminal PRs

Required Checks

1. Verify route access controls are in place ✅

• Sync route (src/app/api/pr-followup/sync/route.ts): Calls authorizeRequest(request) and returns 401 if unauthorized. The pull-based path requires valid auth.
• Webhook route (src/app/api/pr-followup/webhook/route.ts): Uses HMAC-SHA256 signature verification via verifyWebhookSignature with timingSafeEqual (constant-time comparison, preventing timing attacks). Fail-closed by default when WEBHOOK_SECRET is not configured. Auth controls are unchanged and remain in place.

2. Check for unintended public endpoints ✅

• No new endpoints are introduced in this PR. Only the existing POST /api/pr-followup/sync and webhook handler are modified to pass additional PR state fields. The ingestion guard reduces queue noise; it does not open any new access surface.

Standards Compliance

• TypeScript types: Optional prState/prMergedAt fields are typed as string | null | undefined, consistent with the existing PrFollowupEvent interface
• Error handling: Uses the standard error instanceof Error pattern where applicable; this PR adds filtering logic rather than error paths
• No secrets logged: No token or secret values are involved in this change
• PR label conventions: N/A for this PR (no labels involved)
• API route conventions: HTTP status codes are unchanged; sync route returns 401 for unauthorized, 500 for missing GITHUB_TOKEN

Linked Issue Fit

Issue PR 364 explicitly requested filtering at ingest to prevent merged/closed PRs from entering the queue. This PR:

• Adds the guard at the top of both ingestMergeStateEvent and ingestReviewEvent ✅
• Uses cached state/merged_at from the sync run (avoids extra API calls) ✅
• Complements PR PR 362's reconcileStalePrFixItems() cleanup pass with an upstream guard ✅
• All four new test cases verify the guard works for both terminal states (prMergedAt set, prState=closed) ✅

The issue's acceptance criteria are fully met.

Evidence Provider Findings

No evidence providers configured for this PR.

Tool Harness Findings

• git_grep for DISPATCH_AGENT_TOKEN|authorization|bearer returned no matches in the diff, confirming no secrets are being logged or mishandled in this change

CI Check Results

• Validate: completed (success)
• Docker Build: completed (success)

Both CI checks passed before this review began.

Unknowns / Needs Verification

None. The implementation is complete, tested, and the auth controls are verified.